Skip to content

London | 26-ITP-Sept | Fung Nin Lee | Sprint 1 | Sprint-1 exercise - #1496

Open
leerogerfn wants to merge 13 commits into
CodeYourFuture:mainfrom
leerogerfn:Sprint-1
Open

leerogerfn wants to merge 13 commits into
CodeYourFuture:mainfrom
leerogerfn:Sprint-1

Conversation

@leerogerfn

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Task code

CYF-1071

Changelist

completed exercises of Data Group Sprint-1

@leerogerfn leerogerfn added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 5, 2026
@cjyuan cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 5, 2026
Comment thread Sprint-1/fix/median.js Resolved
Comment thread Sprint-1/fix/median.js Resolved
Comment thread Sprint-1/fix/median.js Resolved
Comment thread Sprint-1/implement/dedupe.test.js Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • How should the function handle an array containing NaN?

  • How should the function handle an array containing elements that are not numbers or strings (e.g., null, undefined, true, console (object), etc.)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assessment seems do not require to handle NaN and array which are neither numbers nor strings.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • NaN is also a value of type "number". Handling NaN is optional in this exercise but you should find out how to check if a value is not NaN.

  • The comment in max.test.js does mention "any value that isn't a number"

     // Given an array containing a value that isn't a number
     // When passed to the max function
     // Then it should throw Error("findMax requires an array of numbers")
    

    So the function should also be able to throw an error when the given array contains any value that is not a number.

Comment thread Sprint-1/implement/mean.js Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should calculateMean([Infinity, 0, -Infinity]) or calculateMean([0, NaN]) return?
The same also applies to sum().

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assessment seems do not require to handle the scenario you mentioned.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-Infinity and Infinity are also values of type "number". Change is optional, but you should find out how to check if a number is not one of them.

Comment thread Sprint-1/implement/mean.test.js Resolved
@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Nov 5, 2026
@leerogerfn leerogerfn added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Groups The name of the module. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 6, 2026
Comment thread Sprint-1/fix/median.js
Comment on lines 17 to 19
if (list.some((item) => typeof item === "string" || item === null || item === undefined)) {
throw new Error("calculateMedian requires an array of numbers");
}

@cjyuan cjyuan Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also just check whether item is NOT a value of type number, or even better, check if the item is NOT a finite number.

Even though the test script does not include any test cases involving all possible non-number values ,the comment in the test script does mention:

Arrays containing any non-number value should throw

Comment on lines 23 to 26
function describeMedian(list) {
const result = calculateMedian(list);
return `The median is ${result}`
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The objective of this exercise is to practice using try-catch block to catch and handle the error thrown by a function. calculateMedian() should remain unchanged (per instruction on line 1).

Could you restore the original code of calculateMedian(), and then use a try-catch block in descrubeMedian() accordingly to achieve the objective?

Comment on lines +39 to +43
it("returns for [1, 2, 3, 4]", () => {
const original = [1, 2, 3, 4];
const result = dedupe(original);
expect(result).not.toBe(original);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to checking whether result is a copy of original, this test should also check if result and original contains identical elements.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Data-Groups The name of the module. Reviewed Volunteer to add when completing a review with trainee action still to take. 📅 Sprint 1 Assigned during Sprint 1 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants